Skip to content

Extract BaseRegistrationFormBuilder to share fields across form types#1301

Draft
maebeale wants to merge 14 commits intomainfrom
maebeale/shared-form-builder
Draft

Extract BaseRegistrationFormBuilder to share fields across form types#1301
maebeale wants to merge 14 commits intomainfrom
maebeale/shared-form-builder

Conversation

@maebeale
Copy link
Collaborator

@maebeale maebeale commented Mar 2, 2026

What is the goal of this PR and why is this important?

  • Three form builder services (Short, Extended, Scholarship) duplicated identical helper methods, scholarship fields, and contact fields
  • Changing a shared field (e.g., adding email confirmation) required updating multiple files independently, risking drift
  • Several field key mismatches between builders caused silent bugs in validation and tag assignment

How did you approach the change?

  • Extracted BaseRegistrationFormBuilder with shared helpers (add_header, add_field) and reusable field sections (build_basic_contact_fields, build_scholarship_fields, build_consent_fields)
  • All three builders now inherit from the base class, eliminating ~210 lines of duplication
  • Fixed three bugs discovered during the refactor:
    • Email confirmation key mismatch: Extended form used primary_email_confirmation but controller validation checked confirm_email — confirmation match validation was silently skipped for extended forms
    • workshop_settings vs workshop_environments: Builder created workshop_settings but the service and helper looked for workshop_environments — professional tags were never assigned
    • Redundant confirm email storage: save_form_fields skipped primary_email_confirmation but not confirm_email — short form stored confirmation email unnecessarily
  • Updated email row logic in new.html.erb to support 3-column layout (email | confirm | type) when all three fields exist
  • Added specs for Short and Extended form builders

UI Testing Checklist

  • Short form registration renders correctly with contact, consent, qualitative, and scholarship sections
  • Extended form registration renders correctly with all sections
  • Email confirmation validation works on both form types
  • Email row displays as 3-column on extended form (email | confirm email | email type)
  • Email row displays as 2-column on short form (email | confirm email)
  • Registration show page displays correctly, skipping confirm email field
  • Scholarship application form still builds correctly

Anything else to add?

  • This is forward-only: existing forms in the database are untouched. Only newly built forms will use the shared definitions.
  • The base class is designed for future extensibility — an AuthenticatedEventRegistrationFormBuilder can inherit the same shared sections and add its own fields.

🤖 Generated with Claude Code

maebeale and others added 2 commits March 1, 2026 14:49
…ion forms

- Add confirm email field with server-side match validation
- Group email, confirm email, and email type in one row
- Label as "Email" on short forms, "Primary Email" when secondary exists
- Add address type (Home/Work) on same row as street address
- Add consent and training interest questions (appear on all forms)
- Skip storing confirmation field value in user form submissions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Short, Extended, and Scholarship form builders now inherit from a common
base class that provides shared helpers (add_header, add_field) and
reusable field sections (basic contact, scholarship, consent). This
eliminates duplication and ensures changes to shared fields propagate
to all form types.

Also fixes three bugs:
- Email confirmation key mismatch (primary_email_confirmation → confirm_email)
  so validation now works for extended forms
- workshop_settings → workshop_environments so professional tags get assigned
- confirm_email responses no longer stored redundantly in PersonForm

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@maebeale maebeale changed the title Extract BaseRegistrationFormBuilder to share fields across form types HOLD: Extract BaseRegistrationFormBuilder to share fields across form types Mar 2, 2026
@maebeale maebeale marked this pull request as draft March 8, 2026 18:42
@maebeale maebeale changed the title HOLD: Extract BaseRegistrationFormBuilder to share fields across form types Extract BaseRegistrationFormBuilder to share fields across form types Mar 8, 2026
…dmin CRUD

- Replace 4 hardcoded builders (Base, Short, Extended, Scholarship) with a
  single FormBuilderService that accepts selectable sections
- Rename PersonForm → FormSubmission and PersonFormFormField → FormAnswer
  (tables, models, and all references)
- Add admin FormsController with section interstitial (new) and field
  editor (edit) with drag-reorder via existing sortable_controller
- Snapshot question_text on FormAnswer at submission time for answer
  preservation when fields are later deleted
- Add form-level hide_answered_person_questions and
  hide_answered_form_questions booleans for conditional field visibility
- Add sections JSON column to forms to record builder configuration
- Keep form_fields.status column (still used by workshop logs/reports)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
return
end

@form_fields = @form.form_fields.where(status: :active).reorder(position: :asc)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing status from form_fields bc now they'll either be on the form or removed, no need for soft delete


@person_form = @form.person_forms.find_by(person: person)
unless @person_form
@form_submission = @form.form_submissions.find_by(person: person)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing name from person_forms to form_submissions

@form_fields = @form.form_fields.where(status: :active).reorder(position: :asc)
@responses = @person_form.person_form_form_fields.index_by(&:form_field_id)
@form_fields = @form.form_fields.reorder(position: :asc)
@responses = @form_submission.form_answers.index_by(&:form_field_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing responses from person_form_form_fields to form_answers

rename_column :form_answers, :person_form_id, :form_submission_id

# Add question_text snapshot to form_answers (preserves question at submission time)
add_column :form_answers, :question_text, :string
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding question_text as field on form_answers to store/cache the question at time of having answered the form

maebeale and others added 11 commits March 8, 2026 22:04
The standalone registration forms are dev/test data, not required for
production bootstrapping. Placed before event seeds that depend on them.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Links to the public registration page of the first event using this
form, opening in a new tab.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Renders a disabled preview of the form as it would appear to a
registrant. Works for all forms, including those not linked to events.
Accessible from both the forms index and the form editor.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use the same text-sm text-gray-500 hover:text-gray-700 px-2 py-1
pattern in a right-aligned flex-wrap container.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consent is an opt-in acknowledgment, not a yes/no choice.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Allows changing which builder sections are included on an existing form.
Unchecking a section removes its fields; checking adds default fields at
the end. Preserves existing answers via question_text snapshots.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use explicit SECTION_HEADERS mapping to remove headers by question text
instead of by field_group, which failed when sections shared a group
(e.g. person_identifier and person_contact_info both use "contact").

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…elds

- Rename event_feedback section to marketing with updated field keys
- Add three-category conditional visibility: Scholarship-only, Logged out
  only, Answers on file (covers professional + marketing groups)
- Add slide toggle previews on form show page
- Group-aware drag-and-drop: dragging a section header moves all its fields
- Switch to cocoon for adding/removing fields (replaces server-side add_field)
- Style section headers as bold text, indent child fields
- Replace Delete checkbox with Remove link (matching affiliation pattern)
- Fix nested form issue (button_to inside form_with)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add visibility column (always_ask, scholarship_only, logged_out_only,
  answers_on_file) to form_fields with migration
- Update FormBuilderService with GROUP_VISIBILITY defaults per section
- Replace visibility_select_controller with generic chip_select_controller
  that accepts styles via Stimulus values
- Update public_registrations_controller to filter by visibility column
  instead of hardcoded field_group arrays
- Update form show/edit views to derive toggle conditions from visibility
- Fix new cocoon fields not saving: reject_if blank question on new records
- Add validation error display to form edit page

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both were created but never added to the controller index, so they
weren't loading on the page.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add `one_time` boolean to form_fields for cross-form answer hiding
- Two-tier answer hiding: one-time checks all forms, regular checks within event
- Switch field rows to flexbox with wrap for responsive layout
- Make section header names editable text fields
- New cocoon fields now append to bottom of form field list
- Add ONE_TIME_GROUPS to FormBuilderService for professional/background sections

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant